Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support for Ruby 3.3 #205

Merged
merged 9 commits into from
Jul 23, 2024
Merged

Support for Ruby 3.3 #205

merged 9 commits into from
Jul 23, 2024

Conversation

Geromendez135
Copy link
Contributor

@Geromendez135 Geromendez135 commented Jul 16, 2024 •
edited
Loading

Add support for Ruby 3.3

Found that rubocop-airbnb requires rubocop 1.32.0 which only supports up to Ruby 3.2, See here.
This is the commit that adds support for 3.3 starting from version 1.46.0.

Summary

  • Update rubocop version to support Ruby 3.3
  • Constrain rubocop version to be >= 1.61.0
  • Fixed some deprecations
  • Added Ruby 3.3 to the CI matrix

Test

  • Ran tests locally.

@Geromendez135
update dependencies versions to support 3.3
f1794b2
@Geromendez135
fix error from rubocop 1.48
95b7a6f 
error from said version of rubocop:
RuboCop::ValidationError:
  Property AutoCorrect of cop FactoryBot/CreateList is supposed to be a boolean and contextual is not.

version `1.61` of `rubocop` fixes that issue, allowing all tests to pass.

found that solution in this issue:
rubocop/rubocop-rspec#1916
from rubocop-rspec, where they were experiencing the same
error and updating rubocop to 1.61, solved the issue.
However they don't appear to know the culprit of this.
@Geromendez135
letrubocop version be >= 1.61.0
9c879dc
@Geromendez135
fix deprecation warning
0c794bb 
Warning: obsolete parameter `IgnoredMethods` (for `Style/BlockDelimiters`) found in config/rubocop-style.yml
`IgnoredMethods` has been renamed to `AllowedMethods` and/or `AllowedPatterns`.

parameter `IgnoredMethods` was deprecated in version
`1.33.0` of `rubocop` in favor of `AllowedMethods`. See:
https://github.com/rubocop/rubocop/blob/master/relnotes/v1.33.0.md?plain=1#L19
@Geromendez135
add ruby 3.3 to the CI matrix
3e84360
@Umofomia
Copy link
Contributor

Umofomia commented Jul 19, 2024 •
edited
Loading

It looks like the new workflow for 3.3 is failing because the ruby/setup-ruby snapshot that's being used doesn't have 3.3 support. You'll need to update the following to a newer snapshot of ruby/setup-ruby that has the support:

ruby/.github/workflows/rspec_rubocop.yml

Line 34 in 3e84360

uses: ruby/setup-ruby@ee2113536afb7f793eed4ce60e8d3b26db912da4 # v1.127.0

@Geromendez135
Copy link
Contributor Author

@Umofomia, just updated the setup-ruby snapshot to v1 let me know whether I should use a specific commit sha or that's OK.

@Umofomia
Copy link
Contributor

let me know whether I should use a specific commit sha or that's OK.

I'm fine with using @v1, though wondering if @pariser would like to comment here since he originally introduced the specific commit. The ruby/setup-ruby documentation mentions preferring @v1 anyway, though I can see how a specific commit can be more deterministic.

@Geromendez135
Copy link
Contributor Author

I see it failed for 2.6 because of this breaking change on rubocop-performance
should we remove 2.6 from the tests?

@Umofomia
Copy link
Contributor

It looks like the updates to the dependencies now require Ruby 2.7, so the Ruby 2.6 tests are now failing. We can probably drop support for Ruby 2.6 anyway going forward, so go ahead and remove the 2.6 tests from the matrix:

ruby/.github/workflows/rspec_rubocop.yml

Lines 21 to 22 in bdfad19

- ruby-version: '2.6'
bundler-version: latest

I would also update the line here to indicate that the gem itself only supports 2.7 and higher:

ruby/rubocop-airbnb/rubocop-airbnb.gemspec

Line 18 in bdfad19

spec.required_ruby_version = '>= 2.5'

@Geromendez135 Geromendez135 force-pushed the gm--support_ruby_3.3 branch from 83792bd to 04943b8 Compare July 22, 2024 16:24
Umofomia
Umofomia approved these changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@Umofomia Umofomia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for making the change to add support for Ruby 3.3 to the rubocop-airbnb gem.

@Geromendez135 Geromendez135 force-pushed the gm--support_ruby_3.3 branch from 04943b8 to c200973 Compare July 23, 2024 16:06
@Geromendez135 Geromendez135 force-pushed the gm--support_ruby_3.3 branch from c200973 to 5a11e1a Compare July 23, 2024 16:08
pariser
pariser approved these changes Jul 23, 2024
View reviewed changes
Copy link
Contributor

@pariser pariser left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has:

  • increased the minimum rubocop version to ~> 1.61 (from ~> 1.32.0)
  • dropped support for ruby 2.6
  • added support for ruby 3.3

This has not:

  • changed any of the rubocop rule configurations

Thank you!

@pariser pariser merged commit e62bb1d into airbnb:main Jul 23, 2024
4 checks passed
santiagorodriguez96 added a commit to santiagorodriguez96/ruby that referenced this pull request Oct 17, 2024
@santiagorodriguez96
chore: fix cops with the wrong namespace
33f624e 
As part of the PR for adding support for Ruby 3.3 (airbnb#205) we bumped the
`rubocop-rspec` dependency to `~> 2.26`, which depends on
`rubocop-capybara` `~> 2.17` and `rubocop-factory_bot` `~> 2.22`. On
those versions, the `RSpec` namespace was removed from their cops hence
a warning was being logged when running `rubocop` with the last version
of this gem.

Related PRs:
- rubocop/rubocop-factory_bot@812771e
- rubocop/rubocop-capybara@e36f304
@santiagorodriguez96 santiagorodriguez96 mentioned this pull request Oct 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Umofomia Umofomia Umofomia approved these changes

@pariser pariser pariser approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Geromendez135 @Umofomia @pariser